Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Token generation for prov/consumer mode #2301

Merged
merged 1 commit into from
Dec 20, 2023

Conversation

rchikatw
Copy link
Contributor

@rchikatw rchikatw commented Dec 5, 2023

Creation of a job that is responsible for creating the private and public keys as secret in provider/consumer mode.

  • Added watch on a public secret if it's deleted storage cluster will reconcile
  • Job is created based on public key,

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 5, 2023
@rchikatw rchikatw changed the title Added creation of job template which is responsible for creating the … Creation of job template for onboarding Dec 5, 2023
@rchikatw rchikatw changed the title Creation of job template for onboarding Token generation for prov/consumer mode Dec 5, 2023
@rchikatw rchikatw force-pushed the main branch 2 times, most recently from df3eb91 to e2b9f90 Compare December 11, 2023 06:20
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 11, 2023
go.mod Outdated Show resolved Hide resolved
@rchikatw rchikatw force-pushed the main branch 3 times, most recently from eebdd68 to 86fb2d4 Compare December 11, 2023 07:44
rbac/onboarding-secret-generator-role.yaml Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
@rchikatw rchikatw force-pushed the main branch 2 times, most recently from 738b76b to 6641652 Compare December 11, 2023 09:35
onboarding/main.go Outdated Show resolved Hide resolved
Copy link
Contributor

@leelavg leelavg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Token generation for prov/consumer mode

  • I don't see any token generation code

At the end, This PR will include 2 Commit:
Creation of a job template which is responsible for creating the private and public keys in provider/consumer mode.
Creating RestServer for generation token which will be used for onboarding

  • so the PR is incomplete?
  • what testing is done so far?
  • Pls explain what are you achieving as well in the PR description
  • I stopped the review in the middle due to many discrepancies, I'll relook after a refresh to the PR. Pls excuse.

controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
onboarding/main.go Outdated Show resolved Hide resolved
@rchikatw rchikatw force-pushed the main branch 2 times, most recently from 63b6c32 to 77e62c3 Compare December 12, 2023 18:56
@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2023
@rchikatw rchikatw force-pushed the main branch 2 times, most recently from b0260f4 to 03ab28c Compare December 13, 2023 14:24
@rchikatw rchikatw force-pushed the main branch 2 times, most recently from 4f0f572 to f07bbe6 Compare December 19, 2023 06:45
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/storagecluster_controller.go Outdated Show resolved Hide resolved
hack/source-manifests.sh Outdated Show resolved Hide resolved
rbac/onboarding-secret-generator-role.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@leelavg leelavg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls fix linting errors as well

controllers/storagecluster/provider_server.go Outdated Show resolved Hide resolved
controllers/storagecluster/storagecluster_controller.go Outdated Show resolved Hide resolved
hack/source-manifests.sh Outdated Show resolved Hide resolved
@rchikatw rchikatw force-pushed the main branch 3 times, most recently from 3219b47 to 74785ce Compare December 19, 2023 17:38
Copy link
Contributor

openshift-ci bot commented Dec 19, 2023

@rchikatw: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/ocs-operator-ci 0b165d7 link true /test ocs-operator-ci

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@rchikatw
Copy link
Contributor Author

/retest

Copy link
Contributor

@leelavg leelavg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rchikatw pls get the CI pass.

@iamniting could you pls review and merge if it looks good, Ohad's review isn't required for this PR.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
Added creation of job template which is responsible for
creating the private and public keys in provider mode.

Signed-off-by: rchikatw <[email protected]>
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
@nb-ohad
Copy link
Contributor

nb-ohad commented Dec 20, 2023

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 20, 2023
Copy link
Contributor

openshift-ci bot commented Dec 20, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: leelavg, nb-ohad, rchikatw

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 20, 2023
@openshift-merge-bot openshift-merge-bot bot merged commit 1317c18 into red-hat-storage:main Dec 20, 2023
14 checks passed
@leelavg
Copy link
Contributor

leelavg commented Dec 21, 2023

/cherry-pick release-4.15

@openshift-cherrypick-robot

@leelavg: new pull request created: #2343

In response to this:

/cherry-pick release-4.15

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants